Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow for cropping transparent true color pngs #1878

Merged
merged 17 commits into from
May 25, 2016
Merged

Conversation

Reettaphant
Copy link
Contributor

@Reettaphant Reettaphant commented Apr 28, 2016

Allow cropping pngs with transparency. Optimised the generated assets with pngquant.

Getting this working working locally requires installing pngquant. If installing pngquant with brew, create a symlink from /usr/local/bin/pngquant to /urs/bin/pngquant

@@ -37,9 +37,15 @@ class S3(credentials: AWSCredentials) {
}

private def getContentDispositionFilename(image: Image, charset: CharSet): String = {

val extension = image.source.mimeType match {
case Some("image/jpeg") => "jpg"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Might be better to identify png specifically here and fail in the "other" case.

@kenoir
Copy link
Contributor

kenoir commented Apr 29, 2016

👍 apart from comments.

Let me know if you want to talk through the enum/case class stuff.

@Reettaphant Reettaphant force-pushed the rv-png-alpha branch 2 times, most recently from 9a3a04a to 5081fbe Compare May 5, 2016 14:29
@Reettaphant Reettaphant force-pushed the rv-png-alpha branch 2 times, most recently from cb95383 to b8703b2 Compare May 10, 2016 07:18
@Reettaphant
Copy link
Contributor Author

@NickPapacostas This has now been updated and is ready for review

file: File <- ImageOperations.appendMetadata(strip, metadata)

//Before apps and frontend can handle PNG24s we need to pngquant PNG24 master crops
optimisedFile = if (colourType == "True Color" || colourType == "True Color with Alpha") {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could it ever be True Color? I thought these would go through PNGs-that-should-really-be-JPGs route... Not a problem as such, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I think I forgot all about that. I'll double check and fix it.

@NickPapacostas
Copy link
Contributor

Nice job!

A few little cleanups, then 👍

@Reettaphant Reettaphant merged commit 1383445 into master May 25, 2016
@akash1810 akash1810 deleted the rv-png-alpha branch April 10, 2017 12:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants